Hi,
we are performing an analysis of the RFC-compliance of open-source TLS implementations. Below we list our findings for this implementation. We admit that some are rather nit-picky, but we added them for the sake of completeness. We tried to keep the descriptions brief and didn’t want to spam the report section so feel free to contact us if you have any questions.
If you disagree with our interpretation of certain RFC statements, please leave feedback as we’re interested in your view.

Our results apply to the default configuration of version 3.60. We used the example implementations for client and server.

[S] = Applies to server
[C] = Applies to client
[C+S] = Applies to both

Misc

• [S] NSS enforces that a TLS 1.3 client sends a key share for the first mutually supported group within the supported groups extension. As an example: if a client offers secp521r1 and secp256r1 (in this order) but sends a key share for secp256r1, NSS sends a HelloRetryRequest demanding a secp521r1 key share.

• [C] NSS client does not check for the downgrade bytes a TLS 1.3 server includes in the ServerRandom when negotiating TLS 1.2

  • NSS proceeds to negotiate TLS 1.2 even though it offered TLS 1.3
  • RFC 8446 - 4.1.3 Server Hello

    TLS 1.3 clients receiving a ServerHello indicating TLS 1.2 or below MUST check that the last 8 bytes are not equal to either of these values. If a match is found, the client MUST abort the handshake with an "illegal_parameter" alert.

• [C+S] NSS by default offers and accepts RC4 cipher suites

  • RFC 7465 - 2. Changes to TLS

    TLS clients MUST NOT include RC4 cipher suites in the ClientHello message.

• [C] NSS does not accept an invalid legacy version (such as 0x0304 and 0x0505) set in the ServerHello when TLS 1.3 is negotiated

  • RFC 8446 - 4.2.1 Supported Versions

    If this extension is present, clients MUST ignore the ServerHello.legacy_version value and MUST use only the "supported_versions" extension to determine the selected version.

• [S] upon receiving a Padding extension that contains bytes other than 0x00

  • RFC 7685 - 3. Padding Extension

    The client MUST fill the padding extension completely with zero bytes, although the padding extension_data field may be empty.

Session not aborted

• [C+S] upon receiving a record size exceeds 2^14 + 256 bytes in a TLS 1.3 session, NSS seems to attempt to decrypt the record instead of rejecting it immediately

  • RFC 8446 - 5.2. Record Payload Protection

    The length MUST NOT exceed 2^14 + 256 bytes. An endpoint that receives a record that exceeds this length MUST terminate the connection with a "record_overflow" alert.

• [C+S] upon receiving an interleaved handshake record ("That is, if a handshake message is split over two or more records, there MUST NOT be any other records between them." RFC 8446 - 5. Record Layer) NSS does not always terminate the session

  • Note that we test this using warning alerts that interleave the ServerHello with varying alert descriptions - NSS continues the session for all alert descriptions except for 'Close Notify'
  • RFC 8446 - 5. Record Layer

    Handshake messages MUST NOT be interleaved with other record types.

• [C+S] upon receiving zero-length handshake fragments

  • We tested this by adding an empty handshake record before sending a Hello message
  • RFC 8446 - 6.2.1 Fragmentation

    Implementations MUST NOT send zero-length fragments of Handshake, Alert, or ChangeCipherSpec content types. Zero-length fragments of Application data MAY be sent as they are potentially useful as a traffic analysis countermeasure.

• [S] upon receiving a ClientHello that negotiates TLS 1.3 but does not set the legacy version field to 0x0303

  • RFC 8446 - 4.1.2 Client Hello

    In TLS 1.3, the client indicates its version preferences in the "supported_versions" extension (Section 4.2.1) and the legacy_version field MUST be set to 0x0303, which is the version number for TLS 1.2.

Only session closed but no alert sent

• [C] upon receiving a ServerKeyExchange message that contains a NamedGroup not offered by NSS client, it terminates the connection but does not send an alert if an ECDHE_RSA cipher suite has been selected and the group is not one of the following:

  • brainpool-p512r1, brainpool-p384r1, brainpool-p256r1, ecdh_X448 (secp256r1, secp384r1, secp521r1, and ecdh_X25519 are among the offered named groups)
  • As an example, selecting secp224r1 with TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 would not result in an alert

• [C] upon receiving a close notify alert, NSS does not respond with a close notify

  • RFC 5246 - 7.2.1 Closure Alerts

    The other party MUST respond with a close_notify alert of its own and close down the connection immediately, discarding any pending writes.

Different alert sent than defined by the RFC

• [S] upon receiving a record with an undefined content type at the beginning of the handshake. NSS sends an 'illegal parameter'

  • upon receiving a record with an undefined content type during the handshake, NSS sends a 'decode error'.
  • RFC 8446 - 5. Record Protocol

    If a TLS implementation receives an unexpected record type, it MUST terminate the connection with an "unexpected_message" alert.

• [S] upon receiving a ClientHello that only offers a legacy version not supported by NSS server. NSS sends a 'handshake failure'

  • RFC 5246 - E.1. Compatibility with TLS 1.0/1.1 and SSL 3.0

    If server supports (or is willing to use) only versions greater than client_version, it MUST send a "protocol_version" alert message and close the connection.

• [S] upon receiving a ClientHello with an EcPointFormats extension that only contains compressed or undefined point formats. NSS sends a 'handshake failure'

  • RFC 8422 - 5.1. Client Hello Extensions

    If the client sends the extension and the extension does not contain the uncompressed point format, and the client has used the Supported Groups extension to indicate support for any of the curves defined in this specification, then the server MUST abort the handshake and return an illegal_parameter alert.

• [C] upon receiving a record with invalid content type (0xFF), NSS responds with a 'decode error' alert.

  • RFC 8446 - 5. Record Protocol

    If a TLS implementation receives an unexpected record type, it MUST terminate the connection with an "unexpected_message" alert.

• [C] when a TLS 1.2 server selects a protocol version not supported by NSS client. NSS responds with an 'illegal parameter' alert.

  • RFC 5246 - E.1. Compatibility with TLS 1.0/1.1 and SSL 3.0

    If the version chosen by the server is not supported by the client (or not acceptable), the client MUST send a "protocol_version" alert message and close the connection.

• [C] upon receiving an EncryptedExtensions message that contains extensions that are not permitted in this message, NSS responds with an 'unsupported extension' alert

  • Note that we tested this with Padding and SupportedVersions extensions
  • RFC 8446 - 4.3.1. Encrypted Extensions

    The client MUST check EncryptedExtensions for the presence of any forbidden extensions and if any are found MUST abort the handshake with an "illegal_parameter" alert.

Thanks for raising these issues. I've reviewed these and it looks like there is some work to be done. We will probably want to create separate bugs for each of these, and use this bug to track overall progress. Even though most are fairly trivial changes, they will each require new test cases.

Just a quick note on some of the obvious items:

[S] upon receiving a Padding extension that contains bytes other than 0x00

The specification deliberately doesn't require that the server validate the contents of the extension, so I don't believe that this is a problem.

[C] upon receiving a close notify alert, NSS does not respond with a close notify

You cite RFC 5246 here, which has been updated in RFC 8446 (Section 6.1, paragraph 3). I believe that we are compliant with the updated text, though we will want to look for a test case and add one as necessary. This is one aspect of the protocol where interactions with TCP can interfere with tests. With NSS, it is also possible that PR_Close() was not being invoked correctly in client code; though if this was tstclnt, we should probably fix that.

https://hg.mozilla.org/projects/nss/rev/52137aa125f52c40bda899e31072e6add0ccbbaf

Depends on D122988

Depends on D123535

Comment on attachment 9259106 [details]
Bug 1714579 - Throw illegal_parameter alert for illegal extension in EncryptedExtensions message. r=djackson

Revision D136000 was moved to bug 1751157. Setting attachment 9259106 [details] to obsolete.